refactor(libwaku): async #3180
refactor(libwaku): async #3180
Conversation
richard-ramos
requested review from
darshankabariya,
gabrielmer,
NagyZoltanPeter and
Ivansete-status
|
You can find the image built from this PR at Built from 9a0b46b |
| setupForeignThreadGc() | ||
|
|
||
| body | ||
| template checkLibwakuParams*( |
There was a problem hiding this comment.
I wonder if we should also move this template to the ffi_types module
There was a problem hiding this comment.
Since this template depends on WakuContext we would end up with a import cycle.
library/libwaku.nim
Outdated
| let msg = $error | ||
| callback(RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), userData) | ||
| return nil | ||
| discard waku_thread |
There was a problem hiding this comment.
I think is interesting to handle any possible error, and in case of error, return nil :)
library/libwaku.nim
Outdated
| let res = waku_thread.destroyWakuThread(ctx) | ||
| if res.isErr(): | ||
| foreignThreadGc: | ||
| let msg = "libwaku error: " & $res.error | ||
| callback(RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), userData) | ||
| return RET_ERR |
There was a problem hiding this comment.
Maybe we could use the isOkOr and also return RET_OK if all goes well ;P
| let res = waku_thread.destroyWakuThread(ctx) | |
| if res.isErr(): | |
| foreignThreadGc: | |
| let msg = "libwaku error: " & $res.error | |
| callback(RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), userData) | |
| return RET_ERR | |
| waku_thread.destroyWakuThread(ctx).isOkOr: | |
| foreignThreadGc: | |
| let msg = "libwaku error: " & $error | |
| callback(RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), userData) | |
| return RET_ERR | |
| return RET_OK |
| @@ -28,22 +29,79 @@ type RequestType* {.pure.} = enum | |||
| type InterThreadRequest* = object | |||
There was a problem hiding this comment.
Aside from this PR but I think it would make more sense to rename this type to WakuThreadRequest, if you agree :)
| deallocShared(request) | ||
| let fireSyncRes = request[].doneSignal.fireSync() | ||
| if fireSyncRes.isErr(): | ||
| let msg = "libwaku error: " & $fireSyncRes.error |
There was a problem hiding this comment.
Give a little bit more context
| let msg = "libwaku error: " & $fireSyncRes.error | |
| let msg = "libwaku error: handleRes fireSyncRes error: " & $fireSyncRes.error |
|
|
||
| proc deallocOnDone*(T: type InterThreadRequest, request: ptr InterThreadRequest) = |
There was a problem hiding this comment.
I wonder if we could make this private and invoke it within a defer block inside the handleRes proc.
| proc deallocOnDone*(T: type InterThreadRequest, request: ptr InterThreadRequest) = | |
| proc deallocOnDone(T: type InterThreadRequest, request: ptr InterThreadRequest) = |
There was a problem hiding this comment.
Hm not sure how at the moment. I'm using this deallocOnDone() function as a way to get the functions in libwaku.nim to 'wait' until there's a result and dispose of the waku request afterwards 🤔
| ) | ||
| return | ||
|
|
||
| discard request[].doneSignal.close() |
There was a problem hiding this comment.
Maybe we could handle any possible error
| discard request[].doneSignal.close() | |
| request[].doneSignal.close().isOkOr: | |
| foreignThreadGc: | |
| let msg = "libwaku error: " & $res.error | |
| request[].callback( | |
| RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), request[].userData | |
| ) |
| return | ||
|
|
||
| discard request[].doneSignal.close() | ||
| deallocShared(request) |
There was a problem hiding this comment.
I'd move that line to the top of the proc within a defer block
|
|
||
| proc process*( | ||
| T: type InterThreadRequest, request: ptr InterThreadRequest, waku: ptr Waku | ||
| ) {.async.} = | ||
| echo "Request received: " & $request[].reqType |
There was a problem hiding this comment.
Not related to this PR but I think we can remove this line:)
library/libwaku.nim
Outdated
|
|
||
| proc handleRes[T: string | void]( | ||
| res: Result[T, string], callback: WakuCallBack, userData: pointer | ||
| proc handleSentToChannelRes( |
There was a problem hiding this comment.
It seems as if this proc is no longer needed because we are handling the responses within the waku_thread_request module. I see some room for simplification by completely removing that one :)
There was a problem hiding this comment.
I did a small refactor. Should simplify a bit the code
Description
Allows the execution of libwaku functions asynchronisly instead of executing each request linearly in the event loop of
runWaku.